Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix wild cards handling #9066

Merged
merged 4 commits into from
Dec 13, 2019
Merged

fix wild cards handling #9066

merged 4 commits into from
Dec 13, 2019

Conversation

xiangyan99
Copy link
Member

No description provided.

bryevdv
bryevdv previously approved these changes Dec 10, 2019
bryevdv
bryevdv previously approved these changes Dec 10, 2019
@adxsdk6
Copy link

adxsdk6 commented Dec 11, 2019

Can one of the admins verify this patch?

@praveenkuttappan
Copy link
Member

/azp run azure-sdk-for-python - update_pr

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xiangyan99 xiangyan99 closed this Dec 11, 2019
@xiangyan99 xiangyan99 reopened this Dec 11, 2019
@xiangyan99 xiangyan99 closed this Dec 11, 2019
@xiangyan99 xiangyan99 reopened this Dec 11, 2019
@bryevdv bryevdv mentioned this pull request Dec 11, 2019
@xiangyan99 xiangyan99 closed this Dec 11, 2019
@xiangyan99 xiangyan99 reopened this Dec 12, 2019
@xiangyan99 xiangyan99 closed this Dec 12, 2019
@xiangyan99 xiangyan99 reopened this Dec 12, 2019
@xiangyan99 xiangyan99 closed this Dec 12, 2019
@xiangyan99 xiangyan99 reopened this Dec 12, 2019
@lmazuel lmazuel added AzConfig Client This issue points to a problem in the data-plane of the library. labels Dec 12, 2019
@@ -358,7 +337,7 @@ def test_list_configuration_settings_multi_pages(self):
pass

def test_list_configuration_settings_null_label(self):
items = self.app_config_client.list_configuration_settings(labels=[""])
items = self.app_config_client.list_configuration_settings(label_filter="\0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem right, you used to have code code to replace an empty string with "\0" so customer never see "\0", why not keeping it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We used to escape the char for the user and in the new design, we will let user escape it by themselves. This is a change by design.

Copy link
Contributor

@bryevdv bryevdv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new arg names better 👍

@xiangyan99 xiangyan99 merged commit 3b4cdf9 into master Dec 13, 2019
@xiangyan99 xiangyan99 deleted the azconfig_wild_cards branch December 13, 2019 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants